Injection plugins #2241
Injection plugins #2241
Conversation
src/plugins/injection/README.md
Outdated
| @@ -0,0 +1,325 @@ | |||
| - infos = Information about the injection plugin is below | |||
| - infos/author = Markus Raab <elektra@libelektra.org> | |||
src/plugins/injection/README.md
Outdated
| - infos/recommends = | ||
| - infos/placements = presetstorage | ||
| - infos/status = maintained nodep | ||
| - infos/metadata = inject/randSeed inject/structure/sectionRemove inject/structure/sectionDuplicate inject/structure/sectionReallocate inject/semantic inject/resource inject/typo/transposition inject/typo/insertion inject/typo/caseToggle inject/typo/deletion inject/typo/changechar inject/typo/space inject/domain inject/limit/min inject/limit/max |
There was a problem hiding this comment.
please avoid camel case, we always use / for separation. So it would be inject/rand/seed, ...
| ## Usage | ||
|
|
||
| There up to 6 error types currently with injection types for each. | ||
| 1. Structural errors (e.g. missing sections, parameters in wrong sections, omission necessary parameters in section) |
There was a problem hiding this comment.
Afaik some markdown renderer need a newline in between so that this enumeration is recognized.
src/plugins/injection/README.md
Outdated
| ``` | ||
|
|
||
| * Section Reallocator | ||
| * `inject/structure/sectionReallocate` |
There was a problem hiding this comment.
Isn't this the same as rename?
There was a problem hiding this comment.
Theoretically yes as you just "rename" the path. But behind the scenes it is actually really a key removal and adding on another place. As I could see it is not possible to change the name as elektra complains otherwise.
| [logger] | ||
| inject/semantic/#0 = pyLog | ||
| inject/semantic/#1 = log4net | ||
| inject/semantic/#2 = Log4js |
There was a problem hiding this comment.
Isn't this a capitalization error?
There was a problem hiding this comment.
No, the program expects a logging framework for the java domain but actually the user gives a loggingframework for python/.NET/JS as he might not know the correct one.
|
I changed everything. I also added another section concerning logging as I need to communicate which changes were done to lyrebird. Please take a short look over it if you also think this is a good idea. |
|
|
||
| ## Logging of changes | ||
|
|
||
| To see which changes were applied to the whole KeySet, additional metadata will be written to the rootkey. |
There was a problem hiding this comment.
Description is very short, it is not yet clear how this logging should work. In METADATA.ini logging is already proposed, maybe you can extend this work so that logging is not specific to the inject plugin?
There was a problem hiding this comment.
In METADATA.ini logging is already proposed.
I do not see anything concerning logging in the metadata.ini
There was a problem hiding this comment.
Maybe log is the wrong wording as I just want to backtrace injections which were done (i.e. typo error on setting xy, structure error on key zx, etc.)
There was a problem hiding this comment.
I do not see anything concerning logging in the metadata.ini
Sorry, it was removed in 06e10ca and did not really contain useful information.
Maybe log is the wrong wording as I just want to backtrace injections which were done
Yes, better to change the wording then.
There was a problem hiding this comment.
And why not add the changes directly to the keys where the changes occurred?
There was a problem hiding this comment.
And why not add the changes directly to the keys where the changes occurred?
Because some injections simply remove a whole configuration/ section. It is also much simpler to use when all changes can be found on one key's metadata than searching for all keys if they have an injection on them imo
ghost
added
the
|
Please only set "ready to merge" if the PR compiles. |
|
This whole PR does only exist since you told me to do a PR for large README's so it is easier to comment on. You requested a change:
which I did and as you again told me yourself I should mark the PR as "ready to merge" once you should look at it again. This PR wont get merged anyway so why do you require it to compile?? So what should I do now? Mark it/ write you an email/ label it with something otherwise/ just do an issue ... |
|
You just replaced log -> change, what should I review here? Please request reviews more carefully. The logging strategy should have already been a part of the architecture, so it is now quite complicated to give input about the big picture. @mpranj suggested that we should have information that is passed from plugins to plugins in a separate keyset, similar to the plugin handle and plugin config. Your "logs/changes" would be much better placed there. |
The only "task" you explicitly gave me is to change the wording. I did that and now want to see if the whole concept is finished. How should I tell you to take a whole look on the concept other than labeling the PR?
Where did he suggest that? |
|
@Piankero we discussed it in the meeting last week, but I have not written anything about it in doc/decisions yet. I needed some means of communication between plugins. I proposed to use a KeySet for my scenario and @markus2330 suggested that it can be useful for other plugins too. Plugins would have a handle to a "global" KeySet with the sole purpose of communication between plugins. |
ghost
deleted the
I want to propose a plugin which injects errors in configurations. This is my first idea and some sort of architecture document.
If it is accepted I will also add all information to Metadata.ini and error/specification (so many new metadatas are very timeconsuming).